-
Notifications
You must be signed in to change notification settings - Fork 482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Breaking: support for Hercule transclusion w/ backwards compatibility (WIP) #241
base: master
Are you sure you want to change the base?
Conversation
Regarding the breaking sync API change proposal, I noticed looking at the Travis config, Aglio supports node 0.10. Hercule's method for offering a sync API relies on |
👍 For me I am all up for using Hercule transclusions in Aglio. |
859dc67
to
30548c3
Compare
Thanks @zdne 🎉 Any thoughts from @danielgtaylor ? I've covered everything these changes touch, but the uncovered LOCs remains equal, while covered LOCs decreases, thus the small decrease in coverage. |
@jamesramsay I think this is great. I'd like to hold off on merging just for a bit, get an updated stable release out and then move forward with breaking changes to the API. 👍 |
this looks very interesting. Cheeky bump! |
Internally Hercule tokenises files into content and link tokens. Only the link portion (excluding the identifying surrounding syntax) is parsed. In the simple case this is simply a file path.In this regard, Aglio syntax permits a strict subset of Hercule's with differing surrounding markers. Hercule also has built-in circular dependency detection, to prevent the it hanging when provided with bad input. - Remove: INCLUDE regular expression. - New: add tokenising regular expression, link match function to extend Hercule's tokeniser with support for existing Aglio syntax. - Update: compileFile, render to use hercule.transcludeString. - Beaking: collectPathsSync removed in favour of collectPaths (async). Hercule is written using streams, which are asynchronous. Without extension of Hercule's sync API (childProcess.spawnSync) which is significantly less performant, collectPathsSync becomes collectPaths (async). - Docs: updated collectPaths to reflect async.
I noticed some discussion in the API Blueprint Slack chat room about supporting Hercule syntax in Aglio, and can see how it's unfortunate for new comers to Apiary and API Blueprint to make the choice between Aglio's handy rendering capabilities and Hercule's syntax which play's nicer with Markdown.
Rather than leaving you to work out how to replicate the Hercule syntax or a subset thereof, or add Hercule as a dependency, I thought I'd take look.
I've made a few small changes to Hercule to make using it in this application simpler:
collectPathsSync
): New: ability to pass in pathList variable (closes #196) jamesramsay/hercule#198These changes were quite small, and made it quite easy to drop Hercule into Aglio. I've published these changes to NPM as
3.0.0-alpha.1
(w/next
distribution tag) and a breaking change to the async API to use typical error-first, single-parameter callback style.However, because Hercule is entirely built on stream and the API is dominantly asynchronous, I've made
collectPaths
asynchronous in the PR. Adding the pathList to Hercule's sync API is a little bit more complicated.I'd be interested on your thoughts on this contribution. Thanks for you consideration!
Items that should likely be addressed before merging: